Skip to content

fix(helm): support webapp serviceAccount annotations for IRSA#3429

Merged
nicktrn merged 4 commits intomainfrom
fix/helm-webapp-sa-annotations
Apr 23, 2026
Merged

fix(helm): support webapp serviceAccount annotations for IRSA#3429
nicktrn merged 4 commits intomainfrom
fix/helm-webapp-sa-annotations

Conversation

@nicktrn
Copy link
Copy Markdown
Collaborator

@nicktrn nicktrn commented Apr 23, 2026

Mirrors the existing supervisor.serviceAccount pattern onto webapp so operators can annotate the SA (IRSA eks.amazonaws.com/role-arn, Workload Identity, etc.) or bring their own SA. Without this, webapp.serviceAccount.annotations isn't exposed and operators have to patch the SA out-of-band.

webapp:
  serviceAccount:
    create: true
    name: ""
    annotations:
      eks.amazonaws.com/role-arn: arn:aws:iam::123456789012:role/trigger-webapp

Three pieces, same as supervisor:

  • webapp.serviceAccount.create toggle on the SA block
  • webapp.serviceAccount.annotations + name values
  • trigger-v4.webappServiceAccountName helper, used by the SA, the token-syncer RoleBinding subject, and the Deployment's serviceAccountName

Role + RoleBinding are left unguarded (matching supervisor's shape where rbac.create is a separate toggle from serviceAccount.create) - BYO-SA users take on the responsibility of ensuring the SA they supply has the permissions the RoleBinding grants.

Verified with helm template against default values, an IRSA annotation override, and create: false with a custom name.

@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Apr 23, 2026

⚠️ No Changeset found

Latest commit: f4a5d2a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 23, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: e70afeaf-e0e5-4138-92e1-2d80cd3c6d46

📥 Commits

Reviewing files that changed from the base of the PR and between b9d79e5 and f4a5d2a.

📒 Files selected for processing (3)
  • .github/workflows/pr_checks.yml
  • hosting/k8s/helm/templates/_helpers.tpl
  • hosting/k8s/helm/values.yaml
✅ Files skipped from review due to trivial changes (1)
  • .github/workflows/pr_checks.yml
🚧 Files skipped from review as they are similar to previous changes (1)
  • hosting/k8s/helm/templates/_helpers.tpl
📜 Recent review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (29)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
  • GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
  • GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
  • GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
  • GitHub Check: sdk-compat / Node.js 22.12 (ubuntu-latest)
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
  • GitHub Check: typecheck / typecheck
  • GitHub Check: sdk-compat / Node.js 20.20 (ubuntu-latest)
  • GitHub Check: sdk-compat / Cloudflare Workers
  • GitHub Check: sdk-compat / Bun Runtime
  • GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
  • GitHub Check: sdk-compat / Deno Runtime
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
  • GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
  • GitHub Check: Analyze (python)
  • GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,jsx,tsx,json,md,yaml,yml}

📄 CodeRabbit inference engine (AGENTS.md)

Format code using Prettier before committing

Files:

  • hosting/k8s/helm/values.yaml
🧠 Learnings (4)
📓 Common learnings
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/Chart.yaml:1-18
Timestamp: 2025-06-25T13:18:44.103Z
Learning: For the Trigger.dev Helm chart, the chart name should be "trigger" (not "trigger-v4") since this is the first official chart release. Helper templates should use the actual chart name from .Chart.Name rather than hardcoded prefixes.
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.
📚 Learning: 2025-06-25T13:20:17.174Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values.yaml:22-51
Timestamp: 2025-06-25T13:20:17.174Z
Learning: In the Trigger.dev Helm chart values.yaml, the maintainer prefers to use explicit comprehensive warnings for security-sensitive default values rather than implementing secure-by-default behavior that would fail installation. The project uses deterministic default secrets with clear "TESTING ONLY" warnings and instructions for production deployment.

Applied to files:

  • hosting/k8s/helm/values.yaml
📚 Learning: 2025-06-25T14:14:11.965Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/values-production-example.yaml:95-102
Timestamp: 2025-06-25T14:14:11.965Z
Learning: In the Trigger.dev Helm chart production examples, the maintainer prefers to include initial/bootstrap credentials with clear warnings that they should be changed after first login, rather than requiring external secret references that could complicate initial setup. This follows their pattern of providing working examples with explicit security guidance.

Applied to files:

  • hosting/k8s/helm/values.yaml
📚 Learning: 2025-06-25T13:18:04.827Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 2195
File: hosting/k8s/helm/templates/extra-manifests.yaml:1-4
Timestamp: 2025-06-25T13:18:04.827Z
Learning: In the Trigger.dev v4 Helm chart, the user prefers to rely on documentation and examples in values files rather than implementing defensive coding in templates, particularly for features like extraManifests where proper usage is documented.

Applied to files:

  • hosting/k8s/helm/values.yaml

Walkthrough

Updates Helm chart metadata version to 4.0.6. Adds new trigger-v4.webappServiceAccountName helper and tightens trigger-v4.supervisorServiceAccountName behavior to require an explicit name when creation is disabled. Adds webapp.serviceAccount values (create, name, annotations) and makes the webapp ServiceAccount conditional on those values; references the new helper from ServiceAccount, RoleBinding, and Deployment templates. Adjusts PR workflow to ignore changes under hosting/**.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding support for webapp ServiceAccount annotations for IRSA, which matches the core objective of mirroring the supervisor pattern.
Description check ✅ Passed The description is comprehensive and well-structured, covering what was changed (webapp.serviceAccount pattern), why (IRSA/annotation support), implementation details, and verification steps, though it does not follow the provided template structure.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/helm-webapp-sa-annotations

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 23, 2026

🧭 Helm Chart Prerelease Published

Version: 4.0.6-pr3429.f4a5d2a

Install:

helm upgrade --install trigger \
  oci://ghcr.io/triggerdotdev/charts/trigger \
  --version "4.0.6-pr3429.f4a5d2a"

⚠️ This is a prerelease for testing. Do not use in production.

coderabbitai[bot]

This comment was marked as resolved.

nicktrn added 2 commits April 23, 2026 09:18
…e is empty

Previously the name helpers fell back to "default", causing the
token-syncer RoleBinding to bind secret permissions to the namespace's
default ServiceAccount - silently elevating any workload using it.
Applied to both webapp and supervisor helpers.
@nicktrn nicktrn marked this pull request as ready for review April 23, 2026 09:26
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 1 potential issue.

View 2 additional findings in Devin Review.

Open in Devin Review

Comment thread hosting/k8s/helm/templates/_helpers.tpl
@nicktrn nicktrn added the ready label Apr 23, 2026
@nicktrn nicktrn enabled auto-merge (squash) April 23, 2026 13:01
@nicktrn
Copy link
Copy Markdown
Collaborator Author

nicktrn commented Apr 23, 2026

ready

@nicktrn nicktrn merged commit 87b6716 into main Apr 23, 2026
49 checks passed
@nicktrn nicktrn deleted the fix/helm-webapp-sa-annotations branch April 23, 2026 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants